Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to bind to specific network interfaces via Thunderscope #3235

Open
wants to merge 63 commits into
base: master
Choose a base branch
from

Conversation

itsarune
Copy link
Contributor

Description

Summary of changes:

  • For IPv4 addresses sending/receiving, bind to a specific interface [addresses the problem last year where we needed to launch Thunderscope a few times to bind to the correct IP address]
  • Update UDP listeners/senders to reduce unexplainable crashes in python
  • Thunderscope parameter widget to allow changing binded network interfaces on the fly

How can you test?
Play around with ./tbots.py run thunderscope_main --run_blue --run_diagnostics --disable_communcation

Testing Done

At home with different gamecontroller instances on my home network. The video binds to my local interface first, then an interface that isn't connected to a network and then a home interface that had a gamecontroller instance on it.

IMG_0028.mp4

Resolved Issues

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@itsarune itsarune requested review from williamckha, sauravbanna, a team, aviwad and suchirss and removed request for a team June 26, 2024 06:23
Copy link
Contributor

@nimazareian nimazareian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting this error when I launch AI vs AI. Maybe the new ProtoLogger will fix this issue

2024-07-15 10:19:11,051 - [ERROR] - [Thread-33] - root - (threaded_unix_sender.py).__send_protobuf(65) - Received an invalid proto of type TbotsProto.ThunderbotsConfig
2024-07-15 10:19:11,054 - [ERROR] - [Thread-3] - root - (proto_logger.py).__log_protobufs(143) - Exception detected in ProtoLogger
Traceback (most recent call last):
  File "/home/nima/.cache/bazel/_bazel_nima/d4d8c6a2a61c53de6e279fde073469b4/execroot/__main__/bazel-out/k8-fastbuild/bin/software/thunderscope/thunderscope_main.runfiles/__main__/software/thunderscope/replay/proto_logger.py", line 132, in __log_protobufs
    log_entry = ProtoLogger.create_log_entry(proto, current_time)
  File "/home/nima/.cache/bazel/_bazel_nima/d4d8c6a2a61c53de6e279fde073469b4/execroot/__main__/bazel-out/k8-fastbuild/bin/software/thunderscope/thunderscope_main.runfiles/__main__/software/thunderscope/replay/proto_logger.py", line 157, in create_log_entry
    serialized_proto = base64.b64encode(proto.SerializeToString())
google.protobuf.message.EncodeError: Message TbotsProto.ThunderbotsConfig is missing required fields: ai_config.ai_control_config.network_config
2024-07-15 10:19:12,468 - [INFO] - [MainThread] - root - (proto_configuration_widget.py).update_proto_from_file(142) - Loading protobuf from file: /opt/tbotspython/thunderbots_configuration_proto/default_configuration.proto

@nimazareian
Copy link
Contributor

Actually the error goes away when I delete my /opt/tbotspython/thunderbots_configuration_proto/default_configuration.proto

@itsarune
Copy link
Contributor Author

Yeah that's a known issue whenever parameters.proto changes, it breaks the default_configuration_proto

williamckha
williamckha previously approved these changes Sep 2, 2024
Copy link
Contributor

@williamckha williamckha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM very nice!

*
* @return true if the IP address was found, false otherwise
*/
bool getLocalIp(const std::string& interface, std::string& ip_address, bool ipv4 = true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could return std::optional<std::string> to avoid the out parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, updated

williamckha
williamckha previously approved these changes Sep 3, 2024
Copy link

williamckha
williamckha previously approved these changes Oct 19, 2024
williamckha
williamckha previously approved these changes Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants